Add leather addon #1720
Add leather addon #1720
Conversation
Flamefire
left a comment
Flamefire
left a comment
There was a problem hiding this comment.
Thanks for working on this! I added some inline notes
ottml
force-pushed
the
leather_addon
branch
2 times, most recently
from
2a59a2f to
e51f41e
Compare
|
From my side everything is finished so far. We can begin reviewing the changes. |
|
@Flamefire If you have time it would be nice to get a review :) |
…ly died our died by the hunter)
…s for coins - Order, upgrade, enable/disable - Add armor member to nofSoldier, reduce first armor before hitpoints
- refactor gui to handle adding new stuff more easily
… save game and replays
…t by a catapult (losing his armor)
ottml
left a comment
ottml
left a comment
There was a problem hiding this comment.
I started changing the easy parts first to get down the number of open conversations. It would be nice if you (@Flamefire) can have a look at the applied changes and resolve this conversations if you are fine with the changes.
Ok done. If you apply a suggested diff (best via add-to-batch and apply in 1 commit) on GitHub it will auto-resolve the discussion which is fine if it doesn't require additional changes and no further comment or review. Saves some time, but if in doubt do as you did before so we can take a last look. |
Co-authored-by: Alexander Grund <[email protected]>
…s to separate header to a void duplication
…s to separate header to a void duplication
…s to separate header to a void duplication
…ove not used code where goal is nullptr because this can never happen
…wrong good type for leather addon
…dons checks as bool const, use Gamesettings as parameter instead of player
| SmallFont->Draw(drawPt + DrawPoint(10, -20), "1", FontStyle::CENTER, COLOR_RED); | ||
| } | ||
|
|
||
| LOADER.GetImageN("leather_bobs", leatheraddon::bobIndex[leatheraddon::BobType::DonkeyBoatCarryingArmorWare]) |
There was a problem hiding this comment.
Why is the name "DonkeyBoatCarryingArmorWare"? If it is used here it seems misnamed or wrongly used. Surprising at least
| numSoldiers += 2 * 3; | ||
| numArmoredSoldiers += 2 * 2; | ||
| numHelpers += 2 * 1; |
There was a problem hiding this comment.
Ist needed because we have to add the new ones and the ones subtracted before.
I see: you are basically reusing the array you modified for the sending player for the receiving player and need to undo the change.
Better save the original values as const initialNumSoldiers etc and restore here as numSoldiers = initialNumSoldiers + officerSoldiersTraded. That would read better doesn't it?
Otherwise a wording I find better:
Reuse sender counts for (expected) receiver amounts: Add back the figures we subtracted above and add the received ones -> Add 2x the amount sent
Co-authored-by: Alexander Grund <[email protected]>
Co-authored-by: Alexander Grund <[email protected]>
4 participants
Fixes #1711